Skip to content

Conversation

KoolADE85
Copy link
Contributor

This PR is a small refactor of Textarea and Tooltip components to match the current designs and be Typescript components.

@KoolADE85 KoolADE85 changed the base branch from dev to v4 October 3, 2025 22:38
@KoolADE85 KoolADE85 force-pushed the feature/dcc-refactor-textarea-tooltip branch from 1f7f921 to 6bb0f4a Compare October 3, 2025 22:39
@KoolADE85 KoolADE85 changed the title dcc redesign: TextArea and Tooltip dcc redesign: refactor TextArea and Tooltip Oct 3, 2025
@gvwilson gvwilson added feature something new P1 needed for current cycle labels Oct 7, 2025
@gvwilson
Copy link
Contributor

gvwilson commented Oct 7, 2025

@T4rk1n please review

Comment on lines 23 to 41
const asNumber = (value?: string | number): number | undefined => {
return typeof value === 'string'
? isNaN(parseInt(value, 10))
? undefined
: parseInt(value, 10)
: value;
};
const asBool = (value?: string | boolean): boolean | undefined => {
if (typeof value === 'string') {
if (['true', 'TRUE', 'True'].includes(value)) {
return true;
}
if (['false', 'FALSE', 'False'].includes(value)) {
return false;
}
return undefined;
}
return value;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think those conversion are necessary, the props are transfered in json and bool and number. The prop type should be bool or number but the string interpolation makes it harder on the python type check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your idea here, and pushed up a change that removes string as an allowed type for these props.

However... these existed for API compatibility (the docs say string is allowed) so just want you to be aware of the backwards incompatibility that this introduces (especially for numeric props like cols and maxLength)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new P1 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants